-
Notifications
You must be signed in to change notification settings - Fork 421
fix(event_handler): raise more specific SerializationError exception for unsupported types in data validation #4415
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(event_handler): raise more specific SerializationError exception for unsupported types in data validation #4415
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #4415 +/- ##
===========================================
+ Coverage 96.38% 96.39% +0.01%
===========================================
Files 214 219 +5
Lines 10030 10507 +477
Branches 1846 1944 +98
===========================================
+ Hits 9667 10128 +461
- Misses 259 267 +8
- Partials 104 112 +8 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggested changes in doc to make this more visible and easier to parse, and a note about encoders refactoring to singledispatch
. In the ideal world, most of this logic would be shared this level of serialization would benefit other areas like idempotency
to cut duplication.
Ready to review again @sthulb and @heitorlessa |
looking... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last comment before merging - changing the docs section name.
LGTM. @sthulb I'm gonna resolve the merge conflict and get your approval to merge, as @leandrodamascena is on public holiday today Update: nvm, he's working on it |
|
Issue number: #4300
Summary
Changes
The current implementation of the Data Validation does not natively support serializing some types of objects, including SQLAlchemy model objects and other custom classes that may not have a default serialization mechanism. This can lead to issues when trying to return these objects as the error is confuse.
In this PR, we are adding more robust exception handling and a clear message for cases where the object to be serialized is not supported.
Example of this implementation:
User experience
Before this implementation, the customer received an incomprehensible error like:
Now we've added a clear message in the exception
[ERROR] EncoderError: ('Unable to serializer the object <app.MyClass object at 0x7ffff27f45c0> as it is not a supported type. Error details: [TypeError("\'MyClass\' object is not iterable"), TypeError(\'vars() argument must have __dict__ attribute\')]', 'See: https://docs.powertools.aws.dev/lambda/python/latest/core/event_handler/api_gateway/#serializing-objects')
Checklist
If your change doesn't seem to apply, please leave them unchecked.
Is this a breaking change?
RFC issue number:
Checklist:
Acknowledgment
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.